-
Notifications
You must be signed in to change notification settings - Fork 167
Add Superchain interop message passing #595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Solid!, just one thing that you are probably aware of; the ai assistant seems to be generating crosschain messaging code very well, however the changes aren't getting correctly reflected on the UI, the contract code appears, but the checkboxes are kept unchecked as if nothing happened, they're not in sync with the code changes made by the ai |
content: | ||
'<strong>Important:</strong> Only available on chains in the Superchain. Requires deploying your contract to the same address on every chain in the Superchain. <a class="light-link" href="https://docs.optimism.io/stack/interop/superchain-erc20#requirements" target="_blank" rel="noopener noreferrer">Read more.</a>', | ||
'<strong>Important:</strong> Only available on chains in the Superchain. Requires deploying your contract to the same address on every chain in the Superchain.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed the current comment in this tooltip suggests that same-address deployment across chains is always required for Superchain messaging passing, but given my current understanding I think this isn't true for all scenarios, and that it only applies when it is desired to allow only the same contract to call itself across chains.
I think there may be other valid cross-chain messaging patterns using the Superchain where contracts accept messages from different origin senders than themselves, and those wouldn't need same-address deployment.
Maybe we could adjust the comment to clarify that this requirement is conditional based on the authentication pattern being used?, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed the same address requirement from the tooltip, and reworded the NatSpec message to be clearer in commit 5bb6fe5. Let me know if you think that conveys the points.
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Caution Review the following alerts detected in dependencies. According to your organization's Security Policy, you must resolve all "Block" alerts before proceeding. Learn more about Socket for GitHub.
|
Remaining items: